feat: bulk DEK rotation — re-encrypt a whole database under a fresh data key (#140) - #159
Merged
Conversation
…ata key Closes #140 (I142). Credential rotation and data-key rotation answer different questions, and only one of them was answerable. `add_key`/`rotate_key`/`remove_key` re-wrap the SAME DEK under a different KEK: O(1), superblock only, and the right tool when a passphrase leaks. None of them help when the DEK ITSELF leaks — a process memory dump, a core file, an attached debugger — because the key they re-wrap is the thing that leaked. The CRYPTO-1 fix sharpened the point by documenting it on `rotate_key`; this supplies the operation that sentence pointed at. `Chisel::rekey(path, key, argon2_params)` generates a fresh DEK, re-seals every page under it at the SAME page id (so no pointer in any tree changes, only the bytes beneath it), rebuilds the superblock bank, and replaces the file. Two design points, both recorded in ADR 0018. CRASH SAFETY IS COPY-THEN-RENAME, and it has to be. Shadow paging protects writes that go to NEW pages; this rewrites pages where they already are. A crash halfway through an in-place rotation leaves some pages under the new DEK and some under the old, with the surviving superblock naming one of them — an unrecoverable mix, and exactly the half-state the engine otherwise never produces. The replacement is built in a scratch file beside the original, fsynced, then published with an atomic rename, so the database path only ever names a complete file. IT TAKES A PATH, NOT A HANDLE. The rename leaves any previously-opened descriptor pointing at the original, now-unlinked inode; reads and writes through it would silently target a deleted file. Returning a live handle would be returning that hazard, so there is none to misuse. It collapses the key-slot table to the single supplied credential. That is forced, not chosen: a slot's KEK is derived from its own credential, so wrapping the new DEK for a slot requires the credential that slot belongs to. It is also the safer default — if you are rotating because the DEK leaked, quietly preserving every credential that could reach it is not the goal. `add_key` restores the others, and a test pins that remedy. Exposed through the PyO3 binding as a module-level `chisel.rekey(path, key)`, mirroring `chisel.open()` for the same reason: it names a database by path rather than assuming one is open. The GIL is released for the duration (I156 — this is whole-file I/O plus an Argon2id derivation). Adversarial review found four real defects in the first version, all fixed: * IT FAILED ON HEALTHY DATABASES CARRYING CRASH DEBRIS. Requiring every unit to decrypt looked strict and was actually an availability trap: a process killed mid-commit leaves a TORN unit at a page id the freemap marks free, which shadow paging recovers from by never reading it. Such a database opens and works perfectly — but rekey would refuse it, deterministically, at the one moment it is needed. Undecryptable units are now copied through verbatim. That cannot lose committed data (a live page must decrypt or the database would not open) and preserves the file geometry. * NO TEST PROVED THE DEK CHANGED. `seal` draws a fresh nonce per call, so re-sealing under the SAME key also changes every byte — the whole-file comparison caught a no-op but not a same-key reseal. The new test asserts the old cipher can no longer open a page, and fails when the seal is pointed back at `old_cipher`. * PASSPHRASE ROTATION SILENTLY DOWNGRADED ARGON2 COST. `wrap_into` falls back to the OWASP defaults, so a deliberately hardened database was re-wrapped weaker, with no signal, during the operation you run because you were compromised. Cost parameters are now inherited from the slot the key unlocks unless explicitly overridden. * The scratch-path reclamation unlinked whatever occupied a predictable path. Now matched to the spillway's ownership discipline: remove only a plain file this uid owns with a single link, and refuse anything else. Also corrected ARCHITECTURE.md and THEORY.md, which still said bulk rotation was deferred — five lines from the new text describing it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #140 (I142). Stacked on #158.
Credential rotation and data-key rotation answer different questions, and until now only one was answerable.
rotate_key/remove_keyrekey— this PRadd_key/rotate_key/remove_keyre-wrap the same DEK under a different KEK. None of them help when the DEK itself leaks, because the key they re-wrap is the thing that leaked. The CRYPTO-1 fix (#156) sharpened this by documenting it onrotate_key; this supplies the operation that sentence pointed at.Chisel::rekey(path, key, argon2_params)generates a fresh DEK, re-seals every page under it at the same page id (so no pointer in any tree changes, only the bytes beneath it), rebuilds the superblock bank, and replaces the file.Two design points (ADR 0018)
Crash safety is copy-then-rename, and it has to be. Shadow paging protects writes that go to new pages; this rewrites pages where they already are. A crash halfway through an in-place rotation leaves some pages under the new DEK and some under the old, with the surviving superblock naming one of them — an unrecoverable mix, and exactly the half-state the engine otherwise never produces. The replacement is built in a scratch file beside the original, fsynced, then published with an atomic
rename.It takes a path, not a handle. The rename leaves any previously-opened descriptor pointing at the original, now-unlinked inode. Returning a live handle would be returning that hazard, so there is none to misuse.
It collapses the key-slot table to the supplied credential — forced, not chosen: a slot's KEK is derived from its own credential, so wrapping the new DEK for a slot requires the credential that slot belongs to. It is also the safer default: if you are rotating because the DEK leaked, quietly preserving every credential that could reach it is not the goal.
add_keyrestores the others, and a test pins that remedy.Bindings
Exposed as a module-level
chisel.rekey(path, key), mirroringchisel.open()for the same reason — it names a database by path rather than assuming one is open. GIL released for the duration (I156: whole-file I/O plus an Argon2id derivation).The Swift binding is not covered:
chisel-ffi/isn't on this lineage (see #155).Four real defects the adversarial review found, all fixed
sealdraws a fresh nonce per call, so re-sealing under the same key also changes every byte — the whole-file comparison caught a no-op but not a same-key reseal. The new test asserts the old cipher can no longer open a page, and I verified it fails when the seal is pointed back atold_cipher.wrap_intofalls back to the OWASP defaults, so a deliberately hardened database was re-wrapped weaker, with no signal, during the operation you run because you were compromised. Cost is now inherited from the slot the key unlocks.O_EXCL | O_NOFOLLOW0600.)Verification
ARCHITECTURE.mdandTHEORY.mdcorrected — both still said bulk rotation was deferred, five lines from the new text describing it.